Remove system-defined taxonomies, CONTENT_TAGGING_AUTO by bradenmacdonald · Pull Request #38895 · openedx/openedx-platform

bradenmacdonald · 2026-07-16T00:35:48Z

Description

This PR, along with openedx/openedx-core#654 (which must merge first), will:

Remove "system defined" taxonomies. The only one was the "Languages" taxonomy and we don't believe anyone was using it. However, if someone was using it, it will be converted to a regular taxonomy and continue to work.
Remove the "auto tagging" feature that tags every XBlock with the language of the parent course. This was another half-baked feature and its waffle flag was off by default. I don't believe anyone ever used it.
Implements the "read_only" flag for taxonomies, which makes their tags not editable in the UI (other metadata continues to be editable).
Improves some tests and fixes some flakiness
Fixes what seems like a major issue in the API: the tag_object API would silently fail if the taxonomy was not enabled for the org that owns the content you are trying to tag. As a result, a couple of test cases were not even testing what they thought, because the tags never got created in the first place.

Supporting information

Implements openedx/openedx-core#634

Testing instructions

Verify that the taxonomy editor and content tagging features are working as normal.

Sandbox: https://studio.pr-38895-885846.axim-sandboxes.opencraft.hosting/

Deadline

None

openedx-webhooks · 2026-07-16T00:35:53Z

Thanks for the pull request, @bradenmacdonald!

This repository is currently maintained by @openedx/wg-maintenance-openedx-platform-oncall.

Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review.

🔘 Get product approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

If it does, you'll need to submit a product proposal for your contribution, and have it reviewed by the Product Working Group.
- This process (including the steps you'll need to take) is documented here.
If it doesn't, simply proceed with the next step.

🔘 Provide context

To help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:

Dependencies

This PR must be merged before / after / at the same time as ...
Blockers

This PR is waiting for OEP-1234 to be accepted.
Timeline information

This PR must be merged by XX date because ...
Partner information

This is for a course on edx.org.
Supporting documentation
Relevant Open edX discussion forum threads

🔘 Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

Details

Where can I find more information?

If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources:

When can I expect my changes to be merged?

Our goal is to get community contributions seen and reviewed as efficiently as possible.

However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:

The size and impact of the changes that it introduces
The need for product review
Maintenance status of the parent repository

💡 As a result it may take up to several weeks or months to complete a review and merge your PR.

openedx-webhooks · 2026-07-16T00:35:54Z

Thanks for the pull request, @bradenmacdonald!

This repository is currently maintained by @openedx/wg-maintenance-openedx-platform.

Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review.

🔘 Get product approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

If it does, you'll need to submit a product proposal for your contribution, and have it reviewed by the Product Working Group.
- This process (including the steps you'll need to take) is documented here.
If it doesn't, simply proceed with the next step.

🔘 Provide context

To help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:

Dependencies

This PR must be merged before / after / at the same time as ...
Blockers

This PR is waiting for OEP-1234 to be accepted.
Timeline information

This PR must be merged by XX date because ...
Partner information

This is for a course on edx.org.
Supporting documentation
Relevant Open edX discussion forum threads

🔘 Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

Details

Where can I find more information?

If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources:

When can I expect my changes to be merged?

Our goal is to get community contributions seen and reviewed as efficiently as possible.

However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:

The size and impact of the changes that it introduces
The need for product review
Maintenance status of the parent repository

💡 As a result it may take up to several weeks or months to complete a review and merge your PR.

bradenmacdonald · 2026-07-16T23:20:22Z

@@ -0,0 +1,122 @@
+"""


This was renamed from test_tasks.py. Most of the code was existing code, but I removed all the tests related to auto-tagging and only left in the tests related to auto-deleting tags when the content gets deleted.

jesperhodge · 2026-07-27T19:15:20Z

I'll start by sharing Claude Code review:
"

Before this PR, tagging an object with a taxonomy that isn't enabled for the content's org silently did nothing. This PR fixes that by raising InvalidOrgException. That class subclasses Django's ValidationError, and DRF doesn't automatically convert Django's ValidationError into a 400 response. The new tests check the exception at the Python-API level but not through the REST endpoint. Could you check whether the object-tag endpoint catches it and returns a 400, or whether it would surface as a 500?
This PR removes three entries from .annotation_safe_list.yml (openedx_content.Draft, PublishableEntityVersion, PublishableEntityVersionDependency) that don't look related to taxonomies. Was that intentional, or did it come in from a rebase? If those models still need annotations, the PII check could start failing.
In content_tagging/rules.py, the can_change_taxonomy docstring still says "System definied taxonomies cannot be edited," but the code no longer does that, and the line right below it now says admins can edit any taxonomy. Looks like the docstring was missed in the update (also a small "definied" typo).
This removes the optional object_tag_class parameter from content_tagging's tag_object. It let callers pass an ObjectTag subclass for extra validation. Do you know of any callers that still pass it? (The core openedx_tagging.tag_object keeps this parameter, so only the platform wrapper is affected.)
"

bradenmacdonald · 2026-07-30T02:06:06Z

The new tests check the exception at the Python-API level but not through the REST endpoint. Could you check whether the object-tag endpoint catches it and returns a 400, or whether it would surface as a 500?

Ah, there were indeed a few issues there. That one API was actually calling the upstream API instead of the platform's override version, bypassing org checks for superusers. Fixed in 6891b46

This PR removes three entries from .annotation_safe_list.yml (openedx_content.Draft, PublishableEntityVersion, PublishableEntityVersionDependency) that don't look related to taxonomies. Was that intentional, or did it come in from a rebase? If those models still need annotations, the PII check could start failing.

It was intentional. A recent unrelated refactor on master gave these models inline annotations, so when we pull the new version in to openedx-platform, it now causes a lint failure if we also specify the same annotations in the "safe list" file.

In content_tagging/rules.py, the can_change_taxonomy docstring still says "System definied taxonomies cannot be edited," but the code no longer does that, and the line right below it now says admins can edit any taxonomy. Looks like the docstring was missed in the update (also a small "definied" typo).

I missed that with my own find-and-replace, because of the typo. Fixed.

ObjectTag subclass for extra validation. Do you know of any callers that still pass it? (The core openedx_tagging.tag_object keeps this parameter, so only the platform wrapper is affected.)

Good point. It's a completely unused feature and I've now removed it from the openedx-core PR as well.

Thanks @jesperhodge (and Claude :P)!

jesperhodge · 2026-07-31T14:55:59Z

@@ -1,2 +1,2 @@
 """
 Automatic tagging of content


Suggested change

Handlers for Content Tagging

jesperhodge · 2026-07-31T14:57:12Z

@@ -0,0 +1,122 @@
+"""
+Test for auto-tagging content


Suggested change

Test for auto-tagging content

Test for content tagging handlers

jesperhodge · 2026-07-31T14:57:29Z

+
+
+@skip_unless_cms  # Automatically deleting tags when an object is deleted only applies to the CMS
+class TestAutoTagHandlers(  # type: ignore[misc]


Suggested change

class TestAutoTagHandlers( # type: ignore[misc]

class TestContentTaggingHandlers( # type: ignore[misc]

jesperhodge · 2026-07-31T14:58:19Z

+    LiveServerTestCase
+):
+    """
+    Test if the Course and XBlock tags are automatically created


Suggested change

Test if the Course and XBlock tags are automatically created

Test Content Tag handling when tagged courses or libraries are deleted or restored

jesperhodge · 2026-07-31T15:01:13Z

+
+    If any of the taxonomies are not configured for use by the org that owns
+    the content, this will raise ``InvalidOrgException``, unless you also pass
+    ``ignore_invalid_orgs=True``.


Question, nit (non-blocking):
Just for curiosity, why support an ignore_invalid_orgs flag? The purpose is just to silence the error, or am I misunderstanding that?

jesperhodge · 2026-07-31T15:38:07Z

    #   -r requirements/edx/doc.txt
    #   -r requirements/edx/testing.txt
    #   xblocks-contrib
-openedx-core==1.1.0


Reminder: revert before merge

jesperhodge · 2026-07-31T15:38:11Z

    # via
    #   -r requirements/edx/base.txt
    #   xblocks-contrib
-openedx-core==1.1.0


Reminder: revert before merge

jesperhodge · 2026-07-31T15:38:16Z

    # via
    #   -r requirements/edx/base.txt
    #   xblocks-contrib
-openedx-core==1.1.0


Reminder: revert before merge

jesperhodge · 2026-07-31T15:43:57Z

@@ -227,29 +229,40 @@ def ensure_user_has_can_tag_object_permissions(self, user, tags_data, object_id)

    def update(self, request, *args, **kwargs) -> Response:


Can we specify args / kwargs? I think also maybe relevant for OpenAPI spec. (If this is already done somewhere in the serializer, ignore this.)

jesperhodge · 2026-07-31T16:28:49Z


    def update(self, request, *args, **kwargs) -> Response:
        """
-        Extend the update method to fire CONTENT_OBJECT_ASSOCIATIONS_CHANGED event


Would it make sense to refactor this so the override only contains what's actually different from the parent? Right now update() duplicates the parent's entire body (PATCH rejection, validation, permission check, the final retrieve()) even though the only real difference is how each taxonomy's tags get applied.

Concretely: could we extract the per-taxonomy tagging step — the tag_object() call plus its exception-to-ValidationError translation — into its own small method on the parent (something like _apply_taxonomy_tags), and have the parent's update() call it inside the loop? Then this subclass would only need to override that one method (to call the platform's org-scoped tag_object wrapper and add the extra InvalidOrgException handling), and could drop its update() override entirely, inheriting the parent's version as-is.

That would mean this class stays in sync automatically if the parent's update() changes later, and a reader only has to look at the one method that's genuinely platform-specific rather than diffing two near-identical copies.

bradenmacdonald added the create-sandbox open-craft-grove should create a sandbox environment from this PR label Jul 16, 2026

bradenmacdonald requested review from farhan, irtazaakram and salman2013 as code owners July 16, 2026 00:35

openedx-webhooks added open-source-contribution PR author is not from Axim or 2U core contributor PR author is a Core Contributor (who may or may not have write access to this repo). labels Jul 16, 2026

openedx-webhooks added this to Contributions Jul 16, 2026

github-project-automation Bot moved this to Needs Triage in Contributions Jul 16, 2026

bradenmacdonald force-pushed the braden/remove-taxonomy-extra branch from dd9ff31 to 7b4c37c Compare July 16, 2026 23:07

bradenmacdonald commented Jul 16, 2026

View reviewed changes

bradenmacdonald mentioned this pull request Jul 16, 2026

Remove system-defined taxonomies, replacing with "read_only" flag openedx/openedx-core#654

Open

kaustavb12 added create-sandbox open-craft-grove should create a sandbox environment from this PR and removed create-sandbox open-craft-grove should create a sandbox environment from this PR labels Jul 17, 2026

mphilbrick211 moved this from Needs Triage to Waiting on Author in Contributions Jul 20, 2026

bradenmacdonald added 3 commits July 21, 2026 13:38

feat: remove system-defined taxonomies and content auto tagging

b2e780c

temp: use WIP version of openedx-core

046d958

chore: update PII annotation safelist for new openedx-core version

d998f80

bradenmacdonald force-pushed the braden/remove-taxonomy-extra branch from 7b4c37c to d998f80 Compare July 21, 2026 20:38

bradenmacdonald mentioned this pull request Jul 30, 2026

feat: remove "system defined" taxonomy badge, replace w/ read-only openedx/frontend-app-authoring#3183

Open

7 tasks

feat: update REST API and tests to respect org permissions

6891b46

jesperhodge requested changes Jul 31, 2026

View reviewed changes

	Test for auto-tagging content
	Test for content tagging handlers



		@skip_unless_cms # Automatically deleting tags when an object is deleted only applies to the CMS
		class TestAutoTagHandlers( # type: ignore[misc]

	class TestAutoTagHandlers( # type: ignore[misc]
	class TestContentTaggingHandlers( # type: ignore[misc]

	Test if the Course and XBlock tags are automatically created
	Test Content Tag handling when tagged courses or libraries are deleted or restored

		@@ -227,29 +229,40 @@ def ensure_user_has_can_tag_object_permissions(self, user, tags_data, object_id)

		def update(self, request, args, *kwargs) -> Response:

Uh oh!

Conversation

bradenmacdonald commented Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Supporting information

Testing instructions

Deadline

Uh oh!

openedx-webhooks commented Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openedx-webhooks commented Jul 16, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jesperhodge commented Jul 27, 2026

Uh oh!

bradenmacdonald commented Jul 30, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

bradenmacdonald commented Jul 16, 2026 •

edited

Loading

openedx-webhooks commented Jul 16, 2026 •

edited

Loading